Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace kubectl apply with mesheryctl pattern apply #56

Merged
merged 9 commits into from
Jun 27, 2022

Conversation

hershd23
Copy link
Contributor

@hershd23 hershd23 commented Jun 15, 2022

Description

This PR fixes #20

Notes for Reviewers

Most changes taken from @alphaX86 's work with minor changes

Signed commits

  • Yes, I signed my commits.

@hershd23
Copy link
Contributor Author

@gyohuangxin @leecalcote

@hershd23 hershd23 changed the title Add onboard Replace kubectl apply with mesheryctl pattern apply Jun 15, 2022
@hershd23
Copy link
Contributor Author

All these tests seem to be working for the time being. Changing app onboard to pattern apply for Linkerd and testing for OSM as well

@hershd23
Copy link
Contributor Author

hershd23 commented Jun 15, 2022

Linkerd erroring out while using pattern apply, but working perfectly with app onboard

runner@fv-az38-392:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl pattern apply -f "./emojivoto.yml"
Error: Response Status Code 400, possible Server Error
runner@fv-az38-392:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl app onboard -f "./emojivoto.yml"
app successfully onboarded

runner@fv-az38-392:~/work/meshery-smp-action/meshery-smp-action$ kubectl get pods
NAME                                      READY   STATUS            RESTARTS   AGE
linkerd-destination-7f78f877c9-5vkm7      0/4     PodInitializing   0          30s
linkerd-identity-555666dfcb-49s5c         2/2     Running           0          30s
linkerd-proxy-injector-66456889fd-qmn86   0/2     PodInitializing   0          30s

I remember having a conversation sometime back where @piyushsingariya mentioned that pattern apply only accepts URLs and not path to files on local. Is that still the case?

@hershd23
Copy link
Contributor Author

hershd23 commented Jun 15, 2022

There is a problem with the file we are using for OSM. The pattern file expects a name at the very beginning while this is written like separate k8s objects. @Revolyssup how can we construct a pattern file out of the sample application file linked below for OSM?

(Not a ) Pattern for OSM
https://raw.githubusercontent.com/openservicemesh/osm-docs/main/manifests/apps/bookstore.yaml

Compare it with Istio pattern
https://raw.githubusercontent.com/service-mesh-patterns/service-mesh-patterns/master/samples/bookInfoPattern.yaml

Edit :- Even emojivoto is the same format as osm bookstore. Looks like app onboard is more suitable for this case

@hershd23
Copy link
Contributor Author

runner@fv-az201-566:~/work/meshery-smp-action/meshery-smp-action$ kubectl get pods
No resources found in default namespace.
runner@fv-az201-566:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl system login --provider None
Initiating login...
successfully authenticated
runner@fv-az201-566:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl mesh deploy adapter meshery-linkerd:10001
Verifying prerequisites...
✔ LINKERD
\runner@fv-az201-566:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl app onboard -f "./emojivoto.yml"
app successfully onboarded

runner@fv-az201-566:~/work/meshery-smp-action/meshery-smp-action$ kubectl get pods
NAME                                      READY   STATUS            RESTARTS   AGE
linkerd-destination-d58474474-2qf6r       0/4     PodInitializing   0          10s
linkerd-identity-555666dfcb-qtpsw         0/2     PodInitializing   0          10s
linkerd-proxy-injector-6745db7474-44xl7   0/2     PodInitializing   0          10s

Now app onboard works... but not always the first time around.

https://github.com/hershd23/meshery-smp-action/runs/6904759436?check_suite_focus=true#step:4:70

In this flow, it gives a message that app onboarded successfully yet no pods can be seen when trying to debug. Pods show up when the same steps as before are done again. Now this behaviour doesn't happen always but it doesn't look like app onboard is reliable. @leecalcote any ideas?

@hershd23
Copy link
Contributor Author

Faced this same behaviour this time with OSM.

runner@fv-az42-630:~/work/meshery-smp-action/meshery-smp-action$ kubectl get pods
No resources found in default namespace.
runner@fv-az42-630:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl mesh deploy adapter meshery-osm:10009
Verifying prerequisites...
✔ OPEN_SERVICE_MESH
runner@fv-az42-630:~/work/meshery-smp-action/meshery-smp-action$ mesheryctl app onboard -f "./bookstore.yaml"
app successfully onboarded

runner@fv-az42-630:~/work/meshery-smp-action/meshery-smp-action$ kubectl get pods
NAME                             READY   STATUS     RESTARTS   AGE
osm-bootstrap-79bb65f586-7bmxs   0/1     Running    0          14s
osm-controller-dd77575f4-75g64   0/1     Init:0/1   0          14s
osm-injector-5bb8cc9594-sk7nv    0/1     Init:0/1   0          14s

@hershd23
Copy link
Contributor Author

The good thing is that apart from being a little unreliable application creation is going through.

The funny thing is that though my tests it seems:-
pattern apply only accepts url as input with the -f flag
app onboard only accepts file path on local with the -f flag. Hence in this case the application file has to be downloaded first.

@leecalcote @jared Can we look into this behaviour?

@gyohuangxin
Copy link
Member

@hershd23 Sorry for looking at this late. Can I confirm that, from your test results, pattern apply only works fine with Istio, but has issues with Linkerd or OSM? And app onboard isn't always reliable with Linkerd and OSM?

@hershd23
Copy link
Contributor Author

Yes, because emojivoto for linkerd and OSM demo aren't really pattern files.

https://raw.githubusercontent.com/service-mesh-patterns/service-mesh-patterns/master/samples/bookInfoPattern.yaml
This is a pattern file, bookinfo

https://github.com/openservicemesh/osm-docs/blob/main/manifests/apps/bookstore.yaml
This isn't, just a collection of k8s objects. This requires app onboard.

The pattern files requires a name field at the top, that is the error trace

@hershd23
Copy link
Contributor Author

For all other purposes this works for now, having pattern apply for istio and app onboard for the examples of the rest

@gyohuangxin
Copy link
Member

@hershd23 Thanks for reporting this results.

Did you tried to apply the bookinfo pattern on Linkerd and OSM? I think it's worth a try because:

  1. In our plan, we need the unified pattern for all meshes, so that we can report credible performance tests results.
  2. We're supporting other meshes, they may not have own sample applications.

@leecalcote Do you have any suggestions or answers on whether we can apply the bookinfo pattern on every meshes.

@leecalcote
Copy link
Member

Yes, Meshery's Adapters come with a few different sample applications - https://docs.meshery.io/guides/sample-apps. Ideally, we get a few of these worked into to the tests.

if: ${{ github.event_name == 'workflow_dispatch' }}
steps:
- name: Setup Kubernetes
uses: medyagh/[email protected]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hershd23 Can we still use manusa/[email protected] and use start args: "--cpu 4 --memory 5192" to set the cpu and memory? Because it's better to reduce unnecessary changes in this PR. I suggested it in #48, can you try to prove that everything when using manusa/actions-setup-minikube ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. @hershd23

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/hershd23/meshery-smp-action/runs/7053416986?check_suite_focus=true

Works fine, I had to increase the sleep timer for istio though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@gyohuangxin
Copy link
Member

As this PR works fine, I think we can merge it. However, Istio is using pattern apply, Linkerd and OSM are still using app onboard. We will also meet these issues about which method to deploy our workloads when we add other meshes #52 . So, @hershd23 can you create another issue to discuss how to unify the method of deploying workloads?

@hershd23
Copy link
Contributor Author

Hi @gyohuangxin, recently @Revolyssup and @gr455 explained a way to convert kubernetes manifests into a pattern file.

I'll check if I am able to do that. If so I can also store it with the istio bookinfo pattern, not sure which repository that is but I'll find it and convert everything to pattern apply

@gyohuangxin
Copy link
Member

@hershd23 Good, thanks. If it will take some time, we can merge this first. Then you raise another PR if you find it works.

@hershd23
Copy link
Contributor Author

Sure we can do that. I don't have the merge option with me so feel free to merge.

I'll work on the other bit in a separate PR

@leecalcote leecalcote merged commit 33a35e8 into layer5io:master Jun 27, 2022
@leecalcote leecalcote added the area/performance Performance management label Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/performance Performance management
Development

Successfully merging this pull request may close these issues.

Use mesheryctl not bash
3 participants